Fix/#62 payment response#75
Merged
Merged
Conversation
- AuditLog 엔티티와 Repository, Service 추가 - 요청 IP와 User-Agent 자동 수집 처리 - 공고 생성/수정, 모의 서류 지원 생성 감사로그 기록 - 문항 후보 추가, 문항 선택 저장, 답변 저장 감사로그 기록 - 자소서 분석 실행 결과 감사로그 기록 - 변경 전/후 값을 JSON 문자열로 저장하도록 구현
📝 WalkthroughWalkthroughThis PR introduces a Spring AOP-based audit logging system that annotates write operations across multiple domain services. A new ChangesAudit Logging System
Payment Response JSON Flexibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/jobdri/jobdri_api/domain/payment/dto/toss/TossPaymentConfirmResponse.java (1)
6-13: Consider validating the payment status in the response.The
statusfield is captured but not validated inPaymentService.validateTossResponse(). If the Toss API returns a non-successful status (e.g., "FAILED", "CANCELED") with an HTTP 200 response, the current validation would not catch it, potentially completing a payment that actually failed.Consider adding status validation in
PaymentService.validateTossResponse():private void validateTossResponse(PaymentConfirmRequest request, TossPaymentConfirmResponse response) { if (response == null || !request.orderId().equals(response.orderId()) || !request.paymentKey().equals(response.paymentKey()) || response.totalAmount() != request.amount() || !"DONE".equals(response.status())) { // or check against expected success status throw new GeneralException(GeneralErrorCode.PAYMENT_CONFIRM_FAILED, "결제 승인 응답 검증에 실패했습니다."); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/jobdri/jobdri_api/domain/payment/dto/toss/TossPaymentConfirmResponse.java` around lines 6 - 13, The validateTossResponse check in PaymentService.validateTossResponse currently ignores the Toss response status; update the method to also verify response.status() equals the expected success value (e.g., "DONE") in addition to existing checks (orderId, paymentKey, totalAmount) and throw GeneralException(GeneralErrorCode.PAYMENT_CONFIRM_FAILED, ...) if it does not match; reference TossPaymentConfirmResponse.status(), TossPaymentConfirmResponse.orderId(), TossPaymentConfirmResponse.paymentKey(), and TossPaymentConfirmResponse.totalAmount() when implementing the validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/com/jobdri/jobdri_api/domain/audit/aop/AuditLogAspect.java`:
- Around line 50-57: The catch block in AuditLogAspect that currently rethrows
RuntimeException is causing audit failures to bubble up into business logic;
update the exception handling in AuditLogAspect (the catch(RuntimeException e)
around the audit write) to log the error (as it already does) but do not rethrow
— swallow the exception so the joinPoint/proceeding business flow is unaffected;
keep or enhance the log call for visibility (include the exception) and
optionally emit a metric/event for monitoring instead of throwing.
In
`@src/main/java/com/jobdri/jobdri_api/domain/audit/service/AuditLogService.java`:
- Around line 41-43: The ipAddress resolved from headers must be truncated to
the DB column size before persisting: introduce a MAX_IP_LENGTH constant
(matching the VARCHAR(100)) and apply truncation to the output of
resolveIpAddress wherever used (e.g., the call that currently passes
resolveIpAddress(request) into the Audit log constructor and the other
occurrences in the 53-67 block); ensure you truncate (not just validate) to
MAX_IP_LENGTH the returned String before creating the AuditLog/DTO, similar to
how truncate(resolveUserAgent(request), MAX_USER_AGENT_LENGTH) is used.
---
Nitpick comments:
In
`@src/main/java/com/jobdri/jobdri_api/domain/payment/dto/toss/TossPaymentConfirmResponse.java`:
- Around line 6-13: The validateTossResponse check in
PaymentService.validateTossResponse currently ignores the Toss response status;
update the method to also verify response.status() equals the expected success
value (e.g., "DONE") in addition to existing checks (orderId, paymentKey,
totalAmount) and throw GeneralException(GeneralErrorCode.PAYMENT_CONFIRM_FAILED,
...) if it does not match; reference TossPaymentConfirmResponse.status(),
TossPaymentConfirmResponse.orderId(), TossPaymentConfirmResponse.paymentKey(),
and TossPaymentConfirmResponse.totalAmount() when implementing the validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 34d75390-4102-44c1-a274-131371385507
📒 Files selected for processing (10)
src/main/java/com/jobdri/jobdri_api/domain/analysis/service/AnalysisService.javasrc/main/java/com/jobdri/jobdri_api/domain/analysis/service/QuestionService.javasrc/main/java/com/jobdri/jobdri_api/domain/audit/annotation/AuditLogEvent.javasrc/main/java/com/jobdri/jobdri_api/domain/audit/aop/AuditLogAspect.javasrc/main/java/com/jobdri/jobdri_api/domain/audit/entity/AuditLog.javasrc/main/java/com/jobdri/jobdri_api/domain/audit/repository/AuditLogRepository.javasrc/main/java/com/jobdri/jobdri_api/domain/audit/service/AuditLogService.javasrc/main/java/com/jobdri/jobdri_api/domain/jobposting/service/JobPostingService.javasrc/main/java/com/jobdri/jobdri_api/domain/mockapply/service/MockApplyService.javasrc/main/java/com/jobdri/jobdri_api/domain/payment/dto/toss/TossPaymentConfirmResponse.java
Comment on lines
+50
to
+57
| } catch (RuntimeException e) { | ||
| log.warn("Audit log recording failed. action={}, method={}", | ||
| auditLogEvent.action(), | ||
| joinPoint.getSignature().toShortString(), | ||
| e | ||
| ); | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
Avoid propagating audit write failures to business flow.
Rethrowing here means transient audit issues (serialization/DB) can fail user-facing write operations across all annotated services. This should be best-effort logging unless fail-closed is explicitly required.
Suggested fix
} catch (RuntimeException e) {
log.warn("Audit log recording failed. action={}, method={}",
auditLogEvent.action(),
joinPoint.getSignature().toShortString(),
e
);
- throw e;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/com/jobdri/jobdri_api/domain/audit/aop/AuditLogAspect.java`
around lines 50 - 57, The catch block in AuditLogAspect that currently rethrows
RuntimeException is causing audit failures to bubble up into business logic;
update the exception handling in AuditLogAspect (the catch(RuntimeException e)
around the audit write) to log the error (as it already does) but do not rethrow
— swallow the exception so the joinPoint/proceeding business flow is unaffected;
keep or enhance the log call for visibility (include the exception) and
optionally emit a metric/event for monitoring instead of throwing.
Comment on lines
+41
to
+43
| resolveIpAddress(request), | ||
| truncate(resolveUserAgent(request), MAX_USER_AGENT_LENGTH) | ||
| )); |
There was a problem hiding this comment.
Constrain IP length before persisting audit rows.
ipAddress is stored into a VARCHAR(100) column, but header-derived input is unbounded. A long/spoofed value can trigger a DB length exception and fail the request path.
Suggested fix
public class AuditLogService {
private static final int MAX_USER_AGENT_LENGTH = 500;
+ private static final int MAX_IP_ADDRESS_LENGTH = 100;
@@
auditLogRepository.save(AuditLog.create(
user,
action,
targetType,
targetId,
toJson(beforeValue),
toJson(afterValue),
- resolveIpAddress(request),
+ truncate(resolveIpAddress(request), MAX_IP_ADDRESS_LENGTH),
truncate(resolveUserAgent(request), MAX_USER_AGENT_LENGTH)
));
}Also applies to: 53-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/jobdri/jobdri_api/domain/audit/service/AuditLogService.java`
around lines 41 - 43, The ipAddress resolved from headers must be truncated to
the DB column size before persisting: introduce a MAX_IP_LENGTH constant
(matching the VARCHAR(100)) and apply truncation to the output of
resolveIpAddress wherever used (e.g., the call that currently passes
resolveIpAddress(request) into the Audit log constructor and the other
occurrences in the 53-67 block); ensure you truncate (not just validate) to
MAX_IP_LENGTH the returned String before creating the AuditLog/DTO, similar to
how truncate(resolveUserAgent(request), MAX_USER_AGENT_LENGTH) is used.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 어떤 이유로 PR를 하셨나요?
📋 세부 내용 - 왜 해당 PR이 필요한지 작업 내용을 자세하게 설명해주세요
📸 작업 화면 스크린샷
🚨 관련 이슈 번호 [#62]
Summary by CodeRabbit
Bug Fixes
Chores